Skip to content

Gl switch #578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jul 13, 2018
Merged

Gl switch #578

merged 5 commits into from
Jul 13, 2018

Conversation

@dmt0
Copy link
Contributor

@dmt0 dmt0 commented Jul 12, 2018 •
edited
Loading

closes #565
closes #537
closes #545

  • add "GL Acceleration" switch
  • remove *gl traces from UI
  • rebalance trace selection modal
  • make modal close on Esc key
  • fix icon for scattergl
  • don't display 'Satellite maps' when no mapbox token exists

@dmt0 dmt0 requested a review from nicolaskruchten July 12, 2018 19:18
@dmt0 dmt0 self-assigned this Jul 12, 2018
@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented Jul 12, 2018 •
edited
Loading

  • Label should be "WebGL Acceleration", and the order should be "Off"/"On"
  • switching from scatter to line to area should preserve GL-ness

@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented Jul 12, 2018

Actually let's try Rendering: SVG / WebGL

@dmt0 dmt0 force-pushed the gl-switch branch 5 times, most recently from cd3133c to c9642e2 Compare July 13, 2018 15:34
@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented Jul 13, 2018

  • setting glByDefault without setting makeDefaultTrace should result in a GL trace by default.
  • if in glByDefault mode the user switches to SVG and then clicks on another GL-available trace type, it should stay in SVG mode, and only come back if they switch to a GL-unavailable trace type first.

nicolaskruchten
nicolaskruchten reviewed Jul 13, 2018
View reviewed changes
src/lib/traceTypes.js Outdated
value: 'SPECIALIZED',
label: _('Specialized'),
},
WEB_GL: {
Copy link
Contributor

@nicolaskruchten nicolaskruchten Jul 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rename this to 3D plz? instead of just changing the label?

Copy link
Contributor Author

@dmt0 dmt0 Jul 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can, but it'll be ugly - identifier can't start with a digit, so I'd have to access things like['so'] in a bunch of places...

Copy link
Contributor

@nicolaskruchten nicolaskruchten Jul 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THREE_DEE

@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented Jul 13, 2018

Just one comment in the source, and curious about why the new widget isn't showing up in Percy :)

@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented Jul 13, 2018

💃 but please create an issue for the percy thing :)

@dmt0
Copy link
Contributor Author

dmt0 commented Jul 13, 2018

#582

@dmt0 dmt0 merged commit be5ce2b into master Jul 13, 2018
@dmt0 dmt0 deleted the gl-switch branch July 13, 2018 19:49
@nicolaskruchten nicolaskruchten restored the gl-switch branch July 18, 2018 17:16
@nicolaskruchten nicolaskruchten deleted the gl-switch branch July 18, 2018 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@nicolaskruchten nicolaskruchten nicolaskruchten left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@dmt0 dmt0

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

GL switch Satellite maps without mapbox tokens Trace fold icon is line in scattergl mode

3 participants

@dmt0 @nicolaskruchten